[breakpad] Fix compilation with gcc-13 #26479

artem-ogre · 2025-01-29T13:03:28Z

Summary

Changes to recipe: breakpad/cci.20210521

Motivation

Breakpad would not compile with gcc-13 on Ubuntu 24.02 Noble Numbat

Details

1. Fix missing include

See for reference: https://gcc.gnu.org/gcc-13/porting_to.html

Some C++ Standard Library headers have been changed to no longer include other headers that were being used internally by the library. As such, C++ programs that used standard library components without including the right headers will no longer compile.

The following headers are used less widely in libstdc++ and may need to be included explicitly when compiling with GCC 13:

(for std::string, std::to_string, std::stoi etc.)

<system_error> (for std::error_code, std::error_category, std::system_error).

(for std::int8_t, std::int32_t etc.)

(for std::printf, std::fopen etc.)

(for std::strtol, std::malloc etc.)

In particular header <cstdint> now needs to be included directly to compile with gcc-13

2. Fix `-Wnonnull` error

fclose is called on a null file (see the if-check above it). The patch removes the call to fclose

recipes/breakpad/all/patches/0004-Fix-gcc13-compilation.patch

uilianries · 2025-01-30T15:09:35Z

@artem-ogre Hello and thank you for providing this suggested patch.

Still, the current master branch of Breakpad has no hotfix integrated: https://chromium.googlesource.com/breakpad/breakpad/+/refs/heads/main/src/client/linux/handler/minidump_descriptor.h

I also did not find any occurrence of this patch or bug mentioned in upstream's bug list: https://issues.chromium.org/issues?q=componentid:1629899%2B%20cstdint

Could you please confirm the upstream is aware of this bug somehow?

I can mention Gentoo has a similar patch, but they didn't change the -Wnonnull: https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-util/breakpad/files/breakpad-2022.07.12-gcc13.patch

artem-ogre · 2025-01-30T15:21:59Z

@uilianries
The fclose fix was for this file src/processor/exploitability_linux.cc.
See: 9649377

The master (https://chromium.googlesource.com/breakpad/breakpad/+/refs/heads/main/src/processor/exploitability_linux.cc) does not call fclose. Looks like it was heavily refactored since the snapshot used in the recipe.
This should explain the points you raised. Thanks for the thorough research by the way!

artem-ogre · 2025-01-30T15:23:01Z

The function where fclose was called ExploitabilityLinux::DisassembleBytes does not exist in the version of the file on master.

artem-ogre · 2025-01-30T15:30:39Z

As a side note: You have a lot of experience supporting packaging libraries. You probably have the same opinion that treating warnings as errors is not a great idea when providing a library source to the users (perfectly fine for the development workflow). It is rather unconventional that breakpad does this.

artem-ogre · 2025-01-30T15:36:30Z

@uilianries Here, I found fclose fix in master: https://chromium.googlesource.com/breakpad/breakpad/+/30c7f3cfc11cdbf93a12efbe9d46c66d9785879e%5E%21/

uilianries · 2025-01-30T15:50:59Z

@artem-ogre Thank you for pointing out those sources. I can reproduce your reported error by using GCC13 on Linux: breakpad-20210521-linux-gcc13.log

When applying a patch ONLY to include cstdint is passes for me, no further errors:

breakpad-20210521-linux-gcc13-patched.log

I would request to simplify your patch to only include cstdint. I understand your concern about, but we have several projects using Werror that don't cause problems for us.

uilianries · 2025-01-30T15:55:57Z

@artem-ogre out of curiosity, the Breakpad project offers the newer tag 2023.06.01. Meanwhile, the version maintained in CCI is based on 20210521. Do you need a newer package version?

artem-ogre · 2025-01-30T16:04:56Z

@uilianries
Yes, it builds for your configuration, but it does not build for mine.
It could be that our compilers are not exactly the same versions, or it could be standard library are not the same.
For example your compilation command has -std=gnu++20 and mine does not.
Feel free to diff them yourself:
artem-command.txt
uilian-command.txt

I would appreciate if you could indicate a way forward so that it also builds for my configuration :)
If you are not willing to accept my patch please indicate how else can this be fixed.

artem-ogre · 2025-01-30T16:07:53Z

@artem-ogre out of curiosity, the Breakpad project offers the newer tag 2023.06.01. Meanwhile, the version maintained in CCI is based on 20210521. Do you need a newer package version?

Right now my main concern is to make the existing configuration work with Ubuntu 24.04 🙂
But if there is a more recent version in the conan-center-index I would be happy to look into upgrading the dependency. We only use a fraction of breakpad's functionality.

artem-ogre · 2025-01-30T16:11:24Z

Maybe if I share my profile and compiler info this will help you reproduce the problem?

I'm on Ubuntu 24.04 Noble Numbat.

└─$ g++ --version
g++ (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

My conan profile looks something like this:

[settings]
os=Linux
arch=x86_64
compiler=gcc
compiler.libcxx=libstdc++11
compiler.version=13
build_type=Release

[conf]
tools.system.package_manager:mode = install
tools.system.package_manager:sudo = True

Conan version is 2.11.0

artem-ogre · 2025-01-30T16:14:22Z

Would it be fine applying a patch similar to this one: https://chromium.googlesource.com/breakpad/breakpad/+/30c7f3cfc11cdbf93a12efbe9d46c66d9785879e%5E%21/
and leaving -Werror alone?

uilianries · 2025-01-30T16:17:31Z

@uilianries Yes, it builds for your configuration, but it does not build for mine. It could be that our compilers are not exactly the same versions, or it could be standard library are not the same. For example your compilation command has -std=gnu++20 and mine does not. Feel free to diff them yourself: artem-command.txt uilian-command.txt

I would appreciate if you could indicate a way forward so that it also builds for my configuration :) If you are not willing to accept my patch please indicate how else can this be fixed.

Indeed there is something more peculiar. I started a new Docker container with Ubuntu 24.04 and GCC13 installed. Now I can reproduce your case regarding the fclose:

breakpad-20210521-ubuntu24.04-gcc13.log

uilianries · 2025-01-30T16:25:20Z

Would it be fine applying a patch similar to this one: chromium.googlesource.com/breakpad/breakpad/+/30c7f3cfc11cdbf93a12efbe9d46c66d9785879e%5E%21 and leaving -Werror alone?

Yes, please. I just tested using that patch it really works:

breakpad-patch-fclose.log

Please, simplify your current patch to include cstdint. Then add a second patch, only addressed to this official one.

Thank you for keeping pushing this PR.

See: https://chromium.googlesource.com/breakpad/breakpad/+/30c7f3cfc11cdbf93a12efbe9d46c66d9785879e%5E%21/

artem-ogre · 2025-01-30T16:37:06Z

Done, please check

uilianries

LGTM. Both patches are okay, they are integrated in the upstream or by other package managers.

Patch for fclose prevents -Wnonnull: https://chromium.googlesource.com/breakpad/breakpad/+/30c7f3cfc11cdbf93a12efbe9d46c66d9785879e%5E%21/
Patch for cstdint is integrated in Gentoo: https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-util/breakpad/files/breakpad-2022.07.12-gcc13.patch

Thank you for your PR!

artem-ogre · 2025-01-31T10:05:30Z

@uilianries

I understand your concern about, but we have several projects using Werror that don't cause problems for us.

Just wanted to express my opinion on this: treating warnings as errors for a library provided as source is like having a ticking code-rot bomb. It limits the users in how they can build the library. Just one new warning introduced in a more recent version of the compiler and the bomb goes off :)

I'm rather confident that this breakpad version will break with more recent gcc versions. It happened in the past and will happen in the future.

uilianries · 2025-01-31T13:09:03Z

Just wanted to express my opinion on this: treating warnings as errors for a library provided as source is like having a ticking code-rot bomb. It limits the users in how they can build the library. Just one new warning introduced in a more recent version of the compiler and the bomb goes off :)

I'm rather confident that this breakpad version will break with more recent gcc versions. It happened in the past and will happen in the future.

@artem-ogre Thank you for expressing your concern, indeed it may break in the future. As you commented about using a newer version of Breakpad, it could be included in a patch for -Werror a next PR, to avoid more delays in this one. In case it's a blocker for you right now, please, feel free to share your build log again.

artem-ogre · 2025-02-04T10:48:25Z

Can this be merged? Anything else I can do?

artem-ogre · 2025-02-05T09:20:13Z

Looks like the pre-checks are stuck?

jcar87 · 2025-02-05T09:24:52Z

Looks like the pre-checks are stuck?

Apologies - it failed but did not publish the result. There is an issue with an upstream recipe - will sort this out

artem-ogre changed the title ~~#17954 Fix breakpad compilation with gcc-13~~ Fix breakpad compilation with gcc-13 Jan 29, 2025

artem-ogre changed the title ~~Fix breakpad compilation with gcc-13~~ [breakpad] Fix breakpad compilation with gcc-13 Jan 29, 2025

artem-ogre changed the title ~~[breakpad] Fix breakpad compilation with gcc-13~~ [breakpad] Fix compilation with gcc-13 Jan 29, 2025

jcar87 reviewed Jan 30, 2025

View reviewed changes

recipes/breakpad/all/patches/0004-Fix-gcc13-compilation.patch Outdated Show resolved Hide resolved

conan-io#17954 Fix breakpad compilation with gcc-13

4bb258e

artem-ogre force-pushed the 17954-breakpad-gcc13 branch from 9649377 to 4bb258e Compare January 30, 2025 10:51

jcar87 assigned czoido, uilianries and AbrilRBS Jan 30, 2025

Apply patch from upstream instead of disabling -Werror

52fadf0

See: https://chromium.googlesource.com/breakpad/breakpad/+/30c7f3cfc11cdbf93a12efbe9d46c66d9785879e%5E%21/

uilianries approved these changes Jan 30, 2025

View reviewed changes

AbrilRBS approved these changes Jan 31, 2025

View reviewed changes

artem-ogre mentioned this pull request Jan 31, 2025

[libe57format] Fix compilation with gcc-13 #26481

Merged

jcar87 merged commit 281c563 into conan-io:master Feb 5, 2025
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[breakpad] Fix compilation with gcc-13 #26479

[breakpad] Fix compilation with gcc-13 #26479

artem-ogre commented Jan 29, 2025 •

edited

Loading

uilianries commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 30, 2025

artem-ogre commented Jan 30, 2025

artem-ogre commented Jan 30, 2025

uilianries commented Jan 30, 2025

uilianries commented Jan 30, 2025

artem-ogre commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 30, 2025

artem-ogre commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 30, 2025

uilianries commented Jan 30, 2025

uilianries commented Jan 30, 2025

artem-ogre commented Jan 30, 2025

uilianries left a comment

artem-ogre commented Jan 31, 2025 •

edited

Loading

uilianries commented Jan 31, 2025

artem-ogre commented Feb 4, 2025

artem-ogre commented Feb 5, 2025

jcar87 commented Feb 5, 2025

[breakpad] Fix compilation with gcc-13 #26479

[breakpad] Fix compilation with gcc-13 #26479

Conversation

artem-ogre commented Jan 29, 2025 • edited Loading

Summary

Motivation

Details

1. Fix missing include

2. Fix -Wnonnull error

uilianries commented Jan 30, 2025 • edited Loading

artem-ogre commented Jan 30, 2025 • edited Loading

artem-ogre commented Jan 30, 2025

artem-ogre commented Jan 30, 2025

artem-ogre commented Jan 30, 2025

uilianries commented Jan 30, 2025

uilianries commented Jan 30, 2025

artem-ogre commented Jan 30, 2025 • edited Loading

artem-ogre commented Jan 30, 2025

artem-ogre commented Jan 30, 2025 • edited Loading

artem-ogre commented Jan 30, 2025

uilianries commented Jan 30, 2025

uilianries commented Jan 30, 2025

artem-ogre commented Jan 30, 2025

uilianries left a comment

Choose a reason for hiding this comment

artem-ogre commented Jan 31, 2025 • edited Loading

uilianries commented Jan 31, 2025

artem-ogre commented Feb 4, 2025

artem-ogre commented Feb 5, 2025

jcar87 commented Feb 5, 2025

artem-ogre commented Jan 29, 2025 •

edited

Loading

2. Fix `-Wnonnull` error

uilianries commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 30, 2025 •

edited

Loading

artem-ogre commented Jan 31, 2025 •

edited

Loading